-
Notifications
You must be signed in to change notification settings - Fork 955
Expose ys probs to JNI, Kotlin and Java API #2736
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
WalkthroughThe changes extend the Changes
Sequence DiagramsequenceDiagram
participant Caller
participant OnlineRecognizer
participant JNI
participant NativeLib as Native Lib
participant Result
Caller->>OnlineRecognizer: getResult(stream)
OnlineRecognizer->>JNI: Call native method
JNI->>NativeLib: Invoke C++ recognition
NativeLib->>Result: Compute text, tokens, timestamps, ys_probs
Result-->>NativeLib: Return recognition data
NativeLib-->>JNI: Return object array [text, tokens, timestamps, ys_probs]
JNI->>JNI: Extract 4 elements from array
JNI->>OnlineRecognizer: Return jObjectArray
OnlineRecognizer->>OnlineRecognizer: Construct OnlineRecognizerResult(text, tokens, timestamps, ysProbs)
OnlineRecognizer-->>Caller: Return result with ysProbs field
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Areas requiring attention:
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (2)
sherpa-onnx/jni/online-recognizer.cc (1)
412-415: LGTM: ys_probs array properly created and populated.The JNI code correctly creates and populates the
ys_probsfloat array using standard JNI patterns consistent with the existingtimestampsarray handling.However, consider adding documentation (either in code comments or external documentation) explaining what the
ys_probsvalues represent, their expected range, and how they correspond to the tokens.sherpa-onnx/kotlin-api/OnlineRecognizer.kt (1)
86-86: LGTM: ysProbs field added to data class.The new field is correctly typed and positioned. Note that like
timestamps,FloatArrayuses referential equality in Kotlin data classes, which means twoOnlineRecognizerResultinstances with identical probability arrays won't be considered equal by the generatedequals()method. This is consistent with the existing design.Consider adding KDoc documentation to explain what
ysProbsrepresents.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
sherpa-onnx/java-api/src/main/java/com/k2fsa/sherpa/onnx/OnlineRecognizer.java(1 hunks)sherpa-onnx/java-api/src/main/java/com/k2fsa/sherpa/onnx/OnlineRecognizerResult.java(2 hunks)sherpa-onnx/jni/online-recognizer.cc(2 hunks)sherpa-onnx/kotlin-api/OnlineRecognizer.kt(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
sherpa-onnx/kotlin-api/OnlineRecognizer.kt (1)
sherpa-onnx/java-api/src/main/java/com/k2fsa/sherpa/onnx/OnlineRecognizerResult.java (1)
OnlineRecognizerResult(5-34)
sherpa-onnx/java-api/src/main/java/com/k2fsa/sherpa/onnx/OnlineRecognizerResult.java (1)
sherpa-onnx/kotlin-api/OnlineRecognizer.kt (1)
text(82-88)
sherpa-onnx/java-api/src/main/java/com/k2fsa/sherpa/onnx/OnlineRecognizer.java (1)
sherpa-onnx/kotlin-api/OnlineRecognizer.kt (1)
text(82-88)
🔇 Additional comments (5)
sherpa-onnx/jni/online-recognizer.cc (1)
387-389: LGTM: Array size and documentation updated correctly.The array size has been properly increased to accommodate the new
ys_probsfield, and the comment accurately documents the structure of the result array.sherpa-onnx/kotlin-api/OnlineRecognizer.kt (1)
128-130: LGTM: Result extraction and construction updated correctly.The code properly extracts the
ysProbsarray from the fourth element and constructs the result with all four fields, maintaining consistency with the JNI layer.sherpa-onnx/java-api/src/main/java/com/k2fsa/sherpa/onnx/OnlineRecognizerResult.java (2)
9-16: LGTM: Field and constructor properly implemented.The
ysProbsfield is correctly declared asfinal, and the constructor properly assigns it. The implementation follows the same pattern as the existingtimestampsfield.
30-32: LGTM: Getter method follows existing pattern.The getter is correctly implemented and follows the same pattern as
getTimestamps(). Note that it returns the internal array directly, which allows external modification, but this is consistent with the existing design.sherpa-onnx/java-api/src/main/java/com/k2fsa/sherpa/onnx/OnlineRecognizer.java (1)
69-70: LGTM: Result extraction and construction updated correctly.The code properly extracts the
ysProbsarray from the native result and constructsOnlineRecognizerResultwith the updated four-parameter signature, maintaining consistency with the JNI layer.
Hello. I need to work with probability values for OnlineRecognizer in my Android application, so I expose ys_probs array to JNI and implemented it in Java and in Kotlin API.
Summary by CodeRabbit